Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix #273: Correct format code in debug message #276

Merged
merged 1 commit into from
Nov 7, 2019
Merged

Fix #273: Correct format code in debug message #276

merged 1 commit into from
Nov 7, 2019

Conversation

jphickey
Copy link
Contributor

Describe the contribution
Fixes issue #273
Just adds a typecast to avoid the warning

Testing performed
Build code on i686-rtems4.11 platform, with -Wall -Werror and OS_DEBUG_PRINTF option enabled.

Expected behavior changes
Observe no more warning about the format mismatch.

System(s) tested on:
Ubuntu 18.04.2 LTS 64 bit (build host) using i686-rtems4.11 cross toolchain

Contributor Info
Joseph Hickey, Vantage Systems, Inc.

@jphickey
Fix #273: Correct format code in debug message
56bce53 
Cast the address value so it may be printed as an integer.
skliper
skliper reviewed Oct 29, 2019
View reviewed changes
Copy link
Contributor

@skliper skliper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to report as 64 bit, since address could be up to this size?

ghost
ghost approved these changes Oct 29, 2019
View reviewed changes
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is OK for now. RTEMS supports 3 64 bit architectures (x86_64, Sparc64, and RISCV) and we don't currently have them on our radar. The first RTEMS 64 bit port will probably need to go through and address any items like this.

@jphickey
Copy link
Contributor Author

Would it make sense to report as 64 bit, since address could be up to this size?

As it is unsigned long it will typically be 64 bits on a 64-bit machine anyway. The only (minor) issue is you will not get a bunch of leading zeros. But it should work just fine on a 64-bit machine when that day comes.

@skliper
Copy link
Contributor

skliper commented Oct 29, 2019

Would it make sense to report as 64 bit, since address could be up to this size?

As it is unsigned long it will typically be 64 bits on a 64-bit machine anyway. The only (minor) issue is you will not get a bunch of leading zeros. But it should work just fine on a 64-bit machine when that day comes.

Isn't the format of 0x%08lX only up to 8 digits?

@jphickey
Copy link
Contributor Author

Isn't the format of 0x%08lX only up to 8 digits?

The printf field width (8) is a minimum width. Printf formatters will go beyond the specified width to print the value; they don't truncate, they just expand. So on a 64-bit platform it will just print more digits as needed, it'll work fine.

@skliper
Copy link
Contributor

skliper commented Oct 31, 2019

CCB 20191030 - Code reviewed and approved

@skliper skliper added the CCB:Approved Indicates code review and approval by community CCB label Oct 31, 2019
skliper added a commit that referenced this pull request Oct 31, 2019
@skliper
CCB 20191030: Merge #257 #276 #277 #278
58d4d74 
Fixes #271 #272 #273 #274
Code reviewed and approved at 20191030 CCB
@skliper skliper changed the base branch from master to ic-20191030 November 7, 2019 21:17
@skliper skliper merged commit 1f450b2 into ic-20191030 Nov 7, 2019
@skliper skliper deleted the fix-273-debug-formatcode branch November 7, 2019 21:18
@skliper skliper added this to the 5.1.0 milestone Jun 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skliper skliper skliper approved these changes

Assignees
No one assigned
Labels
CCB:Approved Indicates code review and approval by community CCB
Projects
None yet
Milestone
Bootes
Development

Successfully merging this pull request may close these issues.
2 participants
@jphickey @skliper